Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(operator): include kubearmor installation via helm #1377

Closed
wants to merge 1 commit into from
Closed

fix(operator): include kubearmor installation via helm #1377

wants to merge 1 commit into from

Conversation

Ankurk99
Copy link
Member

With this pull request, KubeArmor will be installed with default configurations using the operator through a single Helm install command.

Purpose of PR?:

Fixes #1374

Does this PR introduce a breaking change?
No

If the changes in this PR are manually verified, list down the scenarios covered::
Installation verified locally on k3s.

Checklist:

@Ankurk99 Ankurk99 self-assigned this Aug 19, 2023
@Ankurk99 Ankurk99 requested a review from nyrahul August 21, 2023 06:18
nyrahul
nyrahul approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@nyrahul nyrahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

DelusionalOptimist
DelusionalOptimist requested changes Aug 25, 2023
View reviewed changes
Copy link
Member

@DelusionalOptimist DelusionalOptimist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL at the suggested comments.

deployments/helm/KubeArmorOperator/README.md Show resolved Hide resolved
deployments/helm/KubeArmorOperator/README.md Outdated Show resolved Hide resolved
deployments/helm/KubeArmorOperator/templates/deployment.yaml Outdated Show resolved Hide resolved
deployments/helm/KubeArmorOperator/templates/serviceaccount.yaml Outdated Show resolved Hide resolved
@Ankurk99 Ankurk99 mentioned this pull request Aug 28, 2023
Closed
achrefbensaad
achrefbensaad requested changes Aug 29, 2023
View reviewed changes
Copy link
Member

@achrefbensaad achrefbensaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a test for kubearmor via the chart ?

@Ankurk99 Ankurk99 force-pushed the issue-1374 branch 2 times, most recently from 75e6d8b to 7b057ea Compare August 29, 2023 12:57
@daemon1024
Copy link
Member

daemon1024 commented Aug 31, 2023 •
edited
Loading

Please include deployments/helm in the workflow paths.

Also update namespace in
https://github.com/kubearmor/KubeArmor/blob/1e57ef20b00cd5419d5a6096af253246d6dd6b12/.github/workflows/ci-test-ginkgo.yml#L76C93-L76C104

@daemon1024 daemon1024 mentioned this pull request Aug 31, 2023
Closed
@daemon1024 daemon1024 mentioned this pull request Sep 19, 2023
Merged
@Ankurk99
Copy link
Member Author

Can you please add a test for kubearmor via the chart ?

Addressed 👍

@Ankurk99
include kubearmor installation via helm
a1a57b3 
With this PR KubeArmor will be installed with default configurations
with the operator by a single helm install command.

Signed-off-by: Ankur Kothiwal <[email protected]>
@ShubhamTatvamasi
Copy link
Contributor

Will it work on ARM based CPU?

DelusionalOptimist
DelusionalOptimist approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@DelusionalOptimist DelusionalOptimist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@Ankurk99
Copy link
Member Author

Will it work on ARM based CPU?

The PR only automates the process of deploying KubeArmor from the Operator. If the Operator installation works fine on ARM then this will also work.
We will need to verify the helm installation on ARM.

achrefbensaad
achrefbensaad approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@achrefbensaad achrefbensaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ankurk99
Copy link
Member Author

We have reconsidered our approach and have opted to stick with the current method of installing the operator first, followed by the kubearmorconfig CR. The reasons are:

  • it will be easier to use custom configurations by just editing the kubearmorconfig CR. This will make it easier to test the custom builds for CI testing.
  • It is generally considered good practice to install an operator that subsequently awaits a Custom Resource to proceed.

@Ankurk99 Ankurk99 closed this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DelusionalOptimist DelusionalOptimist DelusionalOptimist approved these changes

@achrefbensaad achrefbensaad achrefbensaad approved these changes

@ShubhamTatvamasi ShubhamTatvamasi ShubhamTatvamasi approved these changes

@nyrahul nyrahul nyrahul approved these changes

Assignees

@Ankurk99 Ankurk99

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Apply default kubearmor config as part of the Helm installation
6 participants
@Ankurk99 @daemon1024 @ShubhamTatvamasi @nyrahul @achrefbensaad @DelusionalOptimist